-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement optional pretty printed stacktraces #2420
Conversation
FYI, if this turns out to work well enough, we could also consider adding tracing of stacktraces to the deadlock detector. Depends on how slow the stacktrace gathering is in the end... |
Another option is to always build with stacktrace support enabled and make it a runtime option. It would then be disabled by default in a way that it does not create any overhead (bail out immediately from wrapped ABIs) |
macos 10.14.1 (18B75)
|
c95580e
to
9a796ce
Compare
@UdjinM6 Can you try it again if you find time? |
9a796ce
to
0e31cc3
Compare
|
Turned out Mac support was never merged into libbacktrace (ianlancetaylor/libbacktrace#2). But the necessary code is used by the Rust devs in https://github.com/rust-lang-nursery/libbacktrace/tree/rust-snapshot-2018-05-22 (they were also the ones who created the initial PR). I changed backtrace.mk to use this repo instead of the upstream repo until Mac support is officially supported by the upstream version. @UdjinM6 If you find time...one last try...if it still doesn't work I give up for now. |
6ac9e81
to
9ad6b0b
Compare
|
094e71f
to
939ed0d
Compare
So, after some investigation it looks like stacktraces on Mac will require more work in the build system. We need to manually generate .dSYM bundles with debug information to make make libbacktrace work on Mac OS. I'll leave it as it is now, simply don't use A build is running atm that does a test crash. I'll post a link to it later. |
939ed0d
to
aa37167
Compare
c9a9159
to
c876ec7
Compare
c876ec7
to
0a7c6c1
Compare
c1bbc1d
to
c3008c5
Compare
This is currently only useful to extract symbols. It fails to gather stacktraces when compiled with MinGW, so we can only use it to get symbol information from a stack trace which we gathered outside of libbacktrace.
c3008c5
to
c81b1e4
Compare
This is a hack and should only be used for debugging. It works by wrapping the C++ ABI __wrap___cxa_allocate_exception. The wrapper records a backtrace and stores it in a global map. Later the stacktrace can be retrieved with GetExceptionStacktraceStr. This commit also adds handlers to pretty print uncaught exceptions and signals.
c81b1e4
to
2c3a035
Compare
Rebased this branch and reworked a few things. The last Travis build contains a few test crashes so that you can see how stacktraces look like. Unfortunately, the stack traces get all mixed up when walking too deep, but this is something I currently have no good solution for. Reason is that the I've disabled stacktraces on the win32/win64 Travis builds for now, as the added debug info results in binaries with multiple hundred MBs of size, causing the linker to take ages (which times out on Travis). If future mingw versions turn out to perform better, we might enable stacktraces for win32/win64 on Travis. MacOS is not supported for now. I've also updated the PR description to make this clear. I'm going to remove the WIP status now, meaning that I'm happy to see reviews and eventually ACKs :) |
Looks good 👍 |
Otherwise the code at the bottom is never executed when nodes crash, leading to no output of debug.log files on Travis.
2c3a035
to
1a4cac0
Compare
Removed the test crashes and force-pushed |
Shouldn't we also drop |
@UdjinM6 Yepp, should have removed them, which I did now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
utACK
This reverts commit 48d92f1.
This reverts commit 48d92f1.
This solves something that is very much on top of my "Why I hate C++" list. I lost so much time already hunting exceptions/segfaults which only happened sporadically in tests. With this PR, you can add
--enable-stacktraces
to./configure
, which will give you pretty stacktraces on nearly all kinds of crashes that might happen. It also adds better default handling for catched unknown (catch(...)
) exceptions.This works on Linux and Windows (only MinGW based, native and wine). On Linux it uses
backtrace()
fromexecinfo.h
to gather the stack trace andStackWalk64
on Windows. It then useslibbacktrace
to extract symbol information. It is required to have debug information enabled, soconfigure.ac
will add-g
to the compiler flags, but still build with optimization (if--enable-debug
is not given at the same time).For all this to work, I had to "hack" into the C++ ABI by using
-Wl,-wrap
. It wraps a few of the ABI methods and gathers the stacktrace before actually doing the exception handling.GetExceptionStacktraceStr
is then able to retrieve the stacktrace from a catched exception.This method only works on GCC. For Clang based builds, I implemented
dlsym(RTLD_NEXT, ...)
based hooks into the C++ standard libs. This is untested currently as I don't have a Mac to test this. Would be happy if someone could test this and give feedback (@UdjinM6 @nmarley maybe). At least it's able to compile...so I have some hope it works :)MacOS support is not complete yet, as it requires some changes to the build system. We specifically need to generate
.dSYM
bundles which contain the necessary debug symbols. Until then,--enable-stacktraces
will produce many libbacktrace errors in the debug output and not show any useful information about stacktraces.